fix(install): stop leaking temp files when an atomic write fails - #165

Merge-queue ejection: root cause and fix

This PR was ejected from the merge queue when the blocking E2E tests job failed on dash-managed-service-metrics (merge-group run 31102672143, job 92621417125):

FAIL: 'dash-managed-service-metrics' was expected to pass on this host but FAILED — a regression.
Step panicked: TTFT metrics did not appear: timed out after 30s waiting for "50ms".

Root cause

The captured screen in that log shows the dashboard still on Home — the 4 keypress that should have switched to Observe never took effect, so the step waited 30s for metrics on a view it never reached.

The two scenarios that open Observe launch the TUI and send the tab key with no assertion in between, unlike the demo-data journeys which assert the home view first. A key written into the pseudo-terminal before the dashboard is reading input can be consumed by whatever holds the terminal at that moment, and nothing retries it. The step then fails much later, in an assertion about a view the dashboard never left — which is exactly the failure signature above.

This is a latent synchronisation defect in the E2E step, not in this PR's production code: nothing in the download/atomic-write change is reachable from rocm dash. It surfaced here rather than in this PR's own checks because those ran against the older base.

Fix

Send the tab key repeatedly until the Observe chip is actually marked active, so the step depends on the dashboard having acted on the key rather than on it having been ready when the key was written. The helper is restricted to idempotent keys (a tab jump, not a toggle).

Reproduction

Deterministic, by pointing the harness at a wrapper that drains the terminal before exec'ing the real binary — i.e. a dashboard that is not yet reading when the key arrives:

cat > /tmp/wrapper/rocm <<'SH'
#!/bin/sh
stty raw -echo min 0 time 20; dd bs=1 count=16 of=/dev/null 2>/dev/null; stty sane
exec /path/to/target/release/rocm "$@"
SH
ROCM_CLI_BINARY=/tmp/wrapper/rocm cargo xtask e2e -- -n "Observe displays metrics"

Before: 1 scenario (1 failed) with the CI symptom verbatim. After: 1 scenario (1 passed). The sibling scenario (dash-loading-service-status), which uses the same step, also passes under the wrapper.

Verified in addition: full cargo xtask e2e matches the base commit exactly (the two diagnose-* failures are pre-existing on this host, confirmed on ec2bcb32 itself), cargo fmt --all --check, both clippy invocations, cargo xtask manifest --check, and cargo test --workspace --all-targets (only two pre-existing proc_lifecycle failures local to this host).

Note: the guard is exercised by these two scenarios on every run, but the key-loss condition itself is only reproducible with the wrapper above, so there is no automated test that would fail without it.

Re-scoped — please re-review, the existing approval predates this

Heads up that this PR now does substantially less than when it was approved. The
approval at 8c1ec0c was for code that is largely no longer here, and this repo
does not dismiss stale reviews on push, so I have re-requested review rather than
let it merge on a review of code that no longer exists.

Dropped: the download half. #198 (c6b5bb5) landed download_file_streaming
on main, which implements the same idea more completely than this PR did —
sibling .part file, rename on success, plus resume via Range, Content-Range
offset validation, SHA-256 and Content-Length cross-checks, a free-space
preflight, and retry classification. Keeping this PR's version would have meant a
second, weaker download path, so it is gone in favour of main's.

Kept: the write_file_atomically temp leak, which #198 did not touch. Both
copies (apps/rocm/src/therock.rs, apps/rocmd/src/lib.rs) still returned early
on a write or rename failure without removing the scratch file, and the name
embeds a timestamp so each retry leaked a distinct orphan. #198 stopped routing
the SDK tarball through the therock.rs copy, but three callers remain, so both
copies are fixed here.

Split out: the e2e TUI flake fix. The Observe-tab keystroke retry was
unrelated scope creep and is moving to its own PR.

Fixes #158 is now Refs #158. The in-call cleanup gaps are closed, but the
issue's actual complaint — orphans that are never cleaned up — survives. A .part
file left by a hard crash or Ctrl-C is keyed on the process id, so the next run's
cleanup targets a different path and never removes it, and nothing in the tree
sweeps for them. #158 should stay open; I have not touched it.